feat: Add CallGraph struct, and dead-function-removal pass #1796
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1796 +/- ##
==========================================
- Coverage 86.63% 86.61% -0.03%
==========================================
Files 191 194 +3
Lines 34738 34883 +145
Branches 31571 31696 +125
==========================================
+ Hits 30097 30214 +117
- Misses 2940 2961 +21
- Partials 1701 1708 +7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
acl-cqc
changed the title
|
can we deprecate remove_polyfuncs instead so we can include this in a non-breaking release? |
…and suppress warning on re-export
Yes, well, we can, and I have, but I admit I am not singing the praises of the tooling here - deprecate has a "since" tag for tools that might use it, but IIUC I have to guess what the next version number will be (i.e. the version in which it'll be deprecated); and the re-export causes problems (I have to suppress a warning there - I tried putting the |
acl-cqc
changed the title
acl-cqc
changed the title
acl-cqc
force-pushed
the
acl/remove_dead_funcs
branch
from
31a7b44 to
08d9ebc
Compare
acl-cqc
force-pushed
the
acl/remove_dead_funcs
branch
from
08d9ebc to
e29ffa2
Compare
hugr-passes/src/call_graph.rs
Outdated
| fn traverse( | ||
| h: &impl HugrView, | ||
| node: Node, | ||
| enclosing: NodeIndex<u32>, |
There was a problem hiding this comment.
Just for clarification - is enclosing the node in the call graph corresponding to the node node in the hugr or something else?
There was a problem hiding this comment.
It's the (callgraph representation of) the nearest enclosing FuncDefn, so maybe the node's parent or an ancestor. I'll rename...
hugr-passes/src/call_graph.rs
Outdated
| /// * If the Hugr is non-Module-rooted and `entry_points` is non-empty | ||
| /// * If the Hugr is Module-rooted, but does not declare `main`, and `entry_points` is empty | ||
| /// * If the Hugr is Module-rooted, and `entry_points` is non-empty but contains nodes that | ||
| /// are not [FuncDefn](OpType::FuncDefn)s |
There was a problem hiding this comment.
As above, I don't think the second should panic.
I think the interface would be cleaner if entry_points, maybe with a different name: must be FuncDefn or FuncDecl nodes that are immediate children of the root.
Now the first panic goes away, and the third would be an error with the offending nodes.
There was a problem hiding this comment.
Changed to error, indeed. But FuncDefns inside a non-Module are invisible from outside (so unless you're gonna add new stuff inside the root - which you can do, but that's not linking, that's....arbitrary editing), so I've not allowed those as entry_points. I could be persuaded, it'd be easier from a code perspective not to check, but it feels wrong :-!
hugr-passes/src/call_graph.rs
Outdated
| let reachable = reachable_funcs(&CallGraph::new(h), h, entry_points).collect::<HashSet<_>>(); | ||
| let unreachable = h | ||
| .nodes() | ||
| .filter(|n| h.get_optype(*n).is_func_defn() && !reachable.contains(n)) |
There was a problem hiding this comment.
We should remove dead FuncDecls too
|
Did you consider tracking in the Node weight whether it's a FuncDecl or a FuncDefn? |
No, good idea, done |
Closes #1753.
remove_polyfuncsin feat!: Addmonomorphizationpass #1733;remove_polyfuncsis kept but deprecated and untested.CallGraphstruct has no public accessors/etc. yet. I propose to leave this until we have more uses, as I found it hard to expose suitable for using petgraph utilities such asBfs.The longer-term plan is to integrate with #1672 by doing dataflow analysis and transforming to remove
Calls that are locally/dynamically impossible, which will make more functions statically-unreachable and hence removable by this.